Fix index name mutation for identifiers containing dots#213
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #196 where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The fix ensures PostgreSQL quoted identifiers with dots are properly preserved throughout the migration pipeline.
Key changes:
- Replaced the
IsDiffSource()marker method withGetObjectName()to provide consistent name extraction across all diff and IR types - Enhanced
stripSchemaQualifications()regex patterns to avoid matching inside quoted identifiers - Updated plan summary and dump formatter to use source objects directly instead of path parsing
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| testdata/diff/create_index/add_index/new.sql | Adds test case for index with dots in name: "public.idx_users" |
| testdata/diff/create_index/add_index/plan.txt | Expected plan output showing the new index correctly displayed |
| testdata/diff/create_index/add_index/plan.sql | Expected SQL output with properly quoted index name |
| testdata/diff/create_index/add_index/plan.json | Expected JSON output showing the new index in migration plan |
| testdata/diff/create_index/add_index/diff.sql | Expected diff SQL with properly quoted index creation |
| ir/ir.go | Implements GetObjectName() for all IR types to replace the removed IsDiffSource() marker method |
| internal/postgres/desired_state.go | Rewrites stripSchemaQualifications() regex to handle quoted identifiers containing dots |
| internal/plan/plan.go | Adds source tracking for sub-resources and uses GetObjectName() for display |
| internal/dump/formatter.go | Updates formatObjectCommentHeader() to use source object's GetObjectName() instead of path parsing |
| internal/diff/diff.go | Updates DiffSource interface with GetObjectName() method and implements it for all diff types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This fixes an issue where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The bug manifested in three places: 1. Comment header generation in dump output 2. Schema qualification stripping when applying SQL to temporary schemas 3. Plan summary display Changes: - Added GetObjectName() method to DiffSource interface for consistent name extraction - Removed unused IsDiffSource() marker method from interface - Fixed formatObjectCommentHeader() to use Source object instead of path parsing - Rewrote stripSchemaQualifications() regex to avoid matching inside quoted identifiers - Enhanced plan.go to track and use source objects for sub-resources - Added test case for index with dots in name: "public.idx_users" The fix ensures that PostgreSQL quoted identifiers containing dots are properly preserved throughout the dump/plan/apply pipeline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
13e503d to
d8f9ba1
Compare
Updated TestCreateMultiFileOutput to create proper IR objects (Type, Table, Function, View) as Source instead of nil. This allows us to remove the fallback logic in formatObjectCommentHeader() that was extracting names from paths. Changes: - cmd/dump/multifile_test.go: Added ir.Type, ir.Table, ir.Function, ir.View as Source objects for test diffs - internal/dump/formatter.go: Removed fallback to getObjectName(path) since all code paths now provide proper Source objects This makes the code cleaner and ensures all diffs have proper type information available for formatting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
alecthomas
pushed a commit
to alecthomas/pgschema
that referenced
this pull request
Jan 26, 2026
* Fix index name mutation for identifiers containing dots (pgplex#196) This fixes an issue where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The bug manifested in three places: 1. Comment header generation in dump output 2. Schema qualification stripping when applying SQL to temporary schemas 3. Plan summary display Changes: - Added GetObjectName() method to DiffSource interface for consistent name extraction - Removed unused IsDiffSource() marker method from interface - Fixed formatObjectCommentHeader() to use Source object instead of path parsing - Rewrote stripSchemaQualifications() regex to avoid matching inside quoted identifiers - Enhanced plan.go to track and use source objects for sub-resources - Added test case for index with dots in name: "public.idx_users" The fix ensures that PostgreSQL quoted identifiers containing dots are properly preserved throughout the dump/plan/apply pipeline. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Refactor test to use proper Source objects instead of nil fallback Updated TestCreateMultiFileOutput to create proper IR objects (Type, Table, Function, View) as Source instead of nil. This allows us to remove the fallback logic in formatObjectCommentHeader() that was extracting names from paths. Changes: - cmd/dump/multifile_test.go: Added ir.Type, ir.Table, ir.Function, ir.View as Source objects for test diffs - internal/dump/formatter.go: Removed fallback to getObjectName(path) since all code paths now provide proper Source objects This makes the code cleaner and ensures all diffs have proper type information available for formatting. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix #196
This fixes an issue where index names containing dots (e.g., "public.idx_users") were being incorrectly mutated during schema operations. The bug manifested in three places:
Changes:
The fix ensures that PostgreSQL quoted identifiers containing dots are properly preserved throughout the dump/plan/apply pipeline.
🤖 Generated with Claude Code